-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix error message modelchain.infer_temperature_model error to be more informative #1977
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix error message modelchain.infer_temperature_model error to be more informative #1977
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adjust the match text here and I think this is done.
pvlib/modelchain.py
Outdated
f'If Arrays are not used, check that the PVSystem ' | ||
f'attributes `racking_model` and `module_type` ' | ||
f'are valid.') | ||
raise ValueError('could not infer temperature model from ' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
raise ValueError('could not infer temperature model from ' | |
raise ValueError('Could not infer temperature model from ' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@matsuobasho this capitalization change will also require the tests to be updated. See the test failures here: https://github.com/pvlib/pvlib-python/actions/runs/8146219415/job/22264244062?pr=1977#step:9:134
…_model Co-authored-by: Cliff Hansen <[email protected]>
Spelling error correction in error message Co-authored-by: Kevin Anderson <[email protected]>
@matsuobasho the tests on this PR are failing due to the match text needing to be updated as mentioned above. After that I think this PR will be ready to merge. |
@kandersolar thank you for the guidance on this, your message about the capitalization in the tests earlier in the week slipped past me. Will address. |
…specified with the correct error message from infer_tempertaure_model
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @matsuobasho!
docs/sphinx/source/whatsnew
for all changes. Includes link to the GitHub Issue with:issue:`num`
or this Pull Request with:pull:`num`
. Includes contributor name and/or GitHub username (link with:ghuser:`user`
).remote-data
) and Milestone are assigned to the Pull Request and linked Issue.